Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A shared library to pull AWS CostUsage data #109

Closed
wants to merge 10 commits into from
Closed

Conversation

sudo-buddy
Copy link

Please ensure your pull request adheres to the following guidelines:

  • make sure to link the related issues in this description
  • when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

Related Issues

Thanks for contributing!

Copy link

This PR will trigger no release when merged.

nock.cleanAll();
});
it('does not create a new instance if previously initialized', async () => {
const awsCostApiClient = AWSCostApiClient.createFrom({ log: console, region: 'us-east-1' });

Check failure

Code scanning / CodeQL

Property access on null or undefined Error test

The base expression of this property access is always undefined.
expect(awsCostApiClient).to.be.an.instanceof(AWSCostApiClient);
});
it('verify input', async () => {
const awsCostApiClient = AWSCostApiClient.createFrom({ log: console, region: 'us-east-1' });

Check failure

Code scanning / CodeQL

Property access on null or undefined Error test

The base expression of this property access is always undefined.
'AmortizedCost',
],
};
response = awsCostApiClient.getUsageCost(input);

Check failure

Code scanning / CodeQL

Assignment to constant Error test

Assignment to variable response, which is
declared
constant.
'AmortizedCost',
],
};
response = awsCostApiClient.getUsageCost(input);

Check warning

Code scanning / CodeQL

Variable not declared before use Warning test

Variable 'response' is used before its
declaration
.
'AmortizedCost',
],
};
response = awsCostApiClient.getUsageCost(input);

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning test

The value assigned to response here is unused.
Comment on lines +66 to +82
const response = {
ResultsByTime: [
{
TimePeriod: {
Start: '2021-01-01',
End: '2021-02-01',
},
Total: {
AmortizedCost: {
Amount: '0.0000000000',
Unit: 'USD',
},
},
},
],
GroupDefinitions: [],
};

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning test

The initial value of response is unused, since it is always overwritten.
* @returns {GetCostAndUsageResponse}
*/

async getCostUsageData(startDate, endDate, granularity, metrics, groupBy, filter) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the required params here are only service, startDate and endDate. Rest of the params should be constant and handled internally.

An example API call could be like this:

const service = 'S3'; // service code would be better if possible
const startDate = '2024-01-01';
const endDate = '2024-01-31';

const usageCost = await getCostUsageData(service, startDate, endDate);  

// example
console.log(`${service} used $${usageCost} in january 2023`);
// logs: "S3 used $12 in january 2023"

return client;
}

constructor(context) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have AWSCostApiClient.createFrom(context) which takes the helix universalcontext as param and knows about implementation of helix universalcontext.

constructor should not know about helix universalcontext. In this case it should be like:

AWSCostApiClient(region, credentials, log);

chai.use(chaiAsPromised);
const { expect } = chai;

describe('run aws api client', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test does not verify the actual result. We can mock the amazon api behind the service using nock. Example for that: https://github.com/adobe/spacecat-api-service/blob/main/test/support/sqs.test.js#L82

@ekremney
Copy link
Member

@sudo-buddy as a rule of thumb, we put shared code which is used by at least two different components into spacecat-shared. Since this logic will only be used by audit worker, we can completely skip creating a new package here and add the code in the cogs handler in audit worker directly

@solaris007
Copy link
Member

closing in favor of adobe/spacecat-audit-worker#115

@solaris007 solaris007 closed this Feb 2, 2024
@solaris007 solaris007 deleted the SITES-18799 branch February 2, 2024 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants